Replace fixed-duration sleeps after bus events with gather#6803
Merged
Conversation
Several tests use ``await asyncio.sleep(...)`` to "wait for the listener to run" after firing a bus event. The fixed duration is real wall-clock time and the wait can be indeterministic — if the handler chain happens to need slightly more time on a busy CI runner, the assertion races the handler. ``Bus.fire_event`` returns the listener tasks since #6252; capture and ``await asyncio.gather(*tasks)`` instead of sleeping. Touches test_bus.py (the bus tests were poking scheduling instead of verifying their assertions), test_home_assistant_watchdog.py, test_plugin_base.py, addons/test_manager.py, docker/test_addon.py, and test_store_execute_reload.py. Other cleanups in the same spirit: - ``_fire_test_event`` in addons/test_addon.py becomes ``async def`` and gathers the listener tasks itself, so its 17 call sites collapse to a single ``await _fire_test_event(...)``. - The two test_store_execute_reload.py sites that used the private ``_update_connectivity()`` helper are reworked to set the cached connectivity flag directly and fire the event themselves so they can gather the listener tasks the same way. - The two ``sleep(1)`` post-pull drains in docker/test_interface.py collapse to ``sleep(0)`` (handler tasks are already gathered inside pull_image), saving ~2s. - The ``sleep(0.01)`` waits inside ``container_events()`` task bodies (api/test_addons.py, api/test_store.py, backups/test_manager.py) are just one-yield-to-the-parent and become ``sleep(0)``. Switching to ``gather`` exposes a few latent test mocks that were silently swallowing TypeErrors as background-task failures before: - ``CGroup.add_devices_allowed`` is ``async def`` but was patched as a plain MagicMock in docker/test_addon.py — now patched via ``new_callable=AsyncMock``. - The watchdog does ``await (await self.start())`` / ``await (await self.restart())`` because ``App.start`` / ``App.restart`` return ``asyncio.Task``. The mocks in addons/test_addon.py (test_app_watchdog, test_watchdog_on_stop, test_watchdog_during_attach) needed ``AsyncMock(return_value=<settled future>)`` to mirror that shape rather than a plain MagicMock.
agners
force-pushed
the
tests-bus-fire-event-gather
branch
from
f515aca to
24fc47d
Compare
14 tasks
mdegat01
reviewed
May 5, 2026
mdegat01
approved these changes
May 5, 2026
Contributor
mdegat01
left a comment
mdegat01
left a comment
There was a problem hiding this comment.
Good call. I really hated those sleeps, especially the ones that had to be > 0 to make the test work, much better 👍
Per review feedback, the ``await asyncio.gather(*coresys.bus.fire_event(...))`` incantation was scattered across many call sites. Add ``tests.common.fire_bus_event`` that takes the coresys, event and data, fires the event and awaits the spawned listener tasks. Convert all matching sites to use it, including the ``_fire_test_event`` wrapper in addons/test_addon.py which now just builds the ``DockerContainerStateEvent`` and delegates.
Member
Author
|
Hm, there was a test issue in one of the runs: This seems an intermittent problem, nothing this PR touched. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Several tests use
await asyncio.sleep(...)to "wait for the listener to run" after firing a bus event. The fixed duration is real wall-clock time and the wait can be indeterministic — if the handler chain happens to need slightly more time on a busy CI runner, the assertion races the handler.Bus.fire_eventreturns the listener tasks since #6252; await them via a newtests.common.fire_bus_event(coresys, event, data)helper that bundles thegatherso the call sites stay short. Touchestest_bus.py(the bus tests were poking scheduling instead of verifying their assertions),test_home_assistant_watchdog.py,test_plugin_base.py,addons/test_manager.py,docker/test_addon.py, andtest_store_execute_reload.py.Other cleanups in the same spirit:
_fire_test_eventinaddons/test_addon.pybecomesasync defand delegates to the helper, so its 17 call sites collapse to a singleawait _fire_test_event(...).test_store_execute_reload.pysites that used the private_update_connectivity()helper are reworked to set the cached connectivity flag directly and fire the event themselves so they can gather the listener tasks the same way.sleep(1)post-pull drains indocker/test_interface.pycollapse tosleep(0)(handler tasks are already gathered insidepull_image), saving ~2s.sleep(0.01)waits insidecontainer_events()task bodies (api/test_addons.py,api/test_store.py,backups/test_manager.py) are just one-yield-to-the-parent and becomesleep(0).Switching to
gatherexposes a few latent test mocks that were silently swallowing TypeErrors as background-task failures before:CGroup.add_devices_allowedisasync defbut was patched as a plainMagicMockindocker/test_addon.py(nownew_callable=AsyncMock), and the watchdog doesawait (await self.start())/await (await self.restart())becauseApp.start/App.restartreturnasyncio.Task— the mocks inaddons/test_addon.py(test_app_watchdog,test_watchdog_on_stop,test_watchdog_during_attach) neededAsyncMock(return_value=<settled future>)to mirror that shape rather than a plainMagicMock.Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed: